feat(oauth): request explicit scopes instead of * on sign-in#2647
Draft
MattBro wants to merge 3 commits into
Draft
feat(oauth): request explicit scopes instead of * on sign-in#2647MattBro wants to merge 3 commits into
MattBro wants to merge 3 commits into
Conversation
Mirror the scopes PostHog advertises as grantable (the API's OAUTH_SCOPES_SUPPORTED, served at /.well-known/oauth-authorization-server) instead of requesting the "*" wildcard, and bump OAUTH_SCOPE_VERSION 5->6 to force existing users to re-authorize with the narrower set. Keeps the desktop token least-privilege: no privileged or internal scopes, and newly-added scopes are not auto-granted. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
React Doctor found no issues in the changed files. 🎉 Reviewed by React Doctor for commit |
Contributor
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
packages/shared/src/oauth.test.ts:9-194
**Large inline snapshot should move to an external file**
The inline snapshot grew from 3 lines (just `"*"`) to 185 lines. Per the team's style rule, extensive snapshots belong in external `.snap` files — keeping them inline makes diffs noisy and the test file hard to navigate. Vitest supports `toMatchSnapshot()` with an auto-managed `__snapshots__/oauth.test.ts.snap` file that would serve the same guard purpose here.
Reviews (1): Last reviewed commit: "feat(oauth): request explicit scopes ins..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
2 tasks
…snapshot The guard only needs to fail when OAUTH_SCOPES changes so the version gets bumped; snapshotting all ~180 scope strings inline made the diff noisy. Assert version + count + an order-sensitive fingerprint instead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The embedded agent's model calls go through PostHog's LLM gateway, which requires llm_gateway:read (or the legacy "*"). The advertised set excludes it (privileged), so request it explicitly; it's granted via the app's seeded scope ceiling. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The desktop app requests the
*wildcard scope at sign-in, so its OAuth token can do anything the user can. It's the main remaining client still requesting*, which blocks retiring the wildcard server-side.Context
Client-side half of retiring the
*wildcard scope:*wildcard: chore(oauth): retire the * wildcard posthog#60342Depends on two server-side pieces (this PR can't ship before them):
@defaultunion scope ceilings — feat(oauth): @default union scope ceilings posthog#64087. Lets this app's ceiling be["@default", "llm_gateway:read"](the unprivileged default + the one privileged scope) without enumerating a list that drifts.["@default", "llm_gateway:read"]in prod (US + EU), coordinated with this app's release.Changes
OAUTH_SCOPESnow requests the explicit set PostHog advertises as grantable (scopes_supportedat/.well-known/oauth-authorization-server) plusllm_gateway:read, instead of["*"]. The embedded agent reaches most of the product through the PostHog MCPexectool (so it needs the broad unprivileged set) and its model calls go through PostHog's LLM gateway (which requiresllm_gateway:read). This drops only the literal*and the other privileged/internal scopes.OAUTH_SCOPE_VERSION5 → 6, which forces existing installs to re-authorize with the narrower set on next use.Only touches the desktop/shared client (the one on
*). The mobile app is a separate OAuth client and already requests a curated list — left unchanged.Follow-up worth doing: fetch
scopes_supportedfrom the well-known endpoint at sign-in and appendllm_gateway:read, so neither the client list nor the server ceiling has to be hand-maintained.How did you test this?
vitest run src/oauth.test.ts— theOAUTH_SCOPESguard (version + count + fingerprint) passes with the new set.biome checkandtsc --noEmitclean on@posthog/shared; full-repo typecheck ran in the pre-commit hook.Automatic notifications